feat(widget): multi-validator provider cards and validator selection fixes#547
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (39)
💤 Files with no reviewable changes (5)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (27)
📝 WalkthroughWalkthroughThe PR updates multi-validator selection and provider display flows, changes related tests and mocks, and simplifies Vite polyfill wiring and workspace dependency versions. ChangesMulti-validator selection UI
Test and build infrastructure
Sequence Diagram(s)sequenceDiagram
participant ProviderSelectionCard
participant getProviderCardItems
participant ProviderCardsTrigger
participant SelectValidator
ProviderSelectionCard->>getProviderCardItems: providerDetailsArr, selectedValidatorsArr, yieldDto
getProviderCardItems-->>ProviderSelectionCard: ProviderCardItem[]
ProviderSelectionCard->>ProviderCardsTrigger: items, multiSelect, onRemoveValidator, tokenSymbol
ProviderCardsTrigger->>SelectValidator: open selector / manage validators
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)packages/widget/tests/mocks/handlers.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.34][ERROR]: unable to find a config; path packages/widget/tests/mocks/yield-api-handlers.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.14][ERROR]: unable to find a config; path packages/widget/tests/mocks/legacy-api-handlers.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.35][ERROR]: unable to find a config; path
Comment |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/widget/vite/vite.config.base.ts (1)
88-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDropping
satisfies UserConfigremoves a useful type guard.Removing the
satisfies UserConfigwrapper means the returned config object is no longer structurally type-checked againstUserConfig, so typos or invalid keys (e.g. inoptimizeDeps,test, orbuild) will silently pass. Ifmerge(...)widens the type such thatsatisfiesno longer compiles, consider keeping the constraint on the literal config object instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/vite/vite.config.base.ts` at line 88, The Vite config returned by the config builder in vite.config.base.ts should keep structural validation against UserConfig, because removing the satisfies UserConfig check allows invalid or misspelled options to slip through. Update the config object around the merge(...) return so it is still checked against UserConfig, ideally by constraining the literal config object itself or otherwise preserving the satisfies UserConfig guard while keeping the inferred type usable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/widget/src/pages-dashboard/overview/earn-details/components/provider-selection-card.tsx`:
- Around line 163-169: The remove-button aria label in
provider-selection-card.tsx is hardcoded English copy, so it needs to be moved
into i18n with interpolation for the validator name. Update the Box aria-label
to use the shared translation helper, add the new translation key in both
packages/widget/src/translation/English/translations.json and
packages/widget/src/translation/French/translations.json, and apply the same
localization pattern in select-validator-trigger.tsx where the same label is
used.
In
`@packages/widget/src/pages/details/earn-page/components/select-yield-section/select-yield-reward-details.tsx`:
- Line 218: The provider logo list is being truncated to a single item in
select-yield-reward-details.tsx, so the rendering loop cannot show multiple
logos for multi-select cases. Update displayedProviders to preserve the full
selected providers collection instead of wrapping only firstProvider, and make
sure the mapping logic that renders the provider logos continues to work with
the complete array in the select-yield-reward-details component.
---
Nitpick comments:
In `@packages/widget/vite/vite.config.base.ts`:
- Line 88: The Vite config returned by the config builder in vite.config.base.ts
should keep structural validation against UserConfig, because removing the
satisfies UserConfig check allows invalid or misspelled options to slip through.
Update the config object around the merge(...) return so it is still checked
against UserConfig, ideally by constraining the literal config object itself or
otherwise preserving the satisfies UserConfig guard while keeping the inferred
type usable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a3b0b49e-b97b-42e9-af96-551472c32835
⛔ Files ignored due to path filters (7)
packages/examples/with-vite-bundled/public/vite.svgis excluded by!**/*.svgpackages/examples/with-vite-bundled/src/typescript.svgis excluded by!**/*.svgpackages/widget/src/assets/images/porto.svgis excluded by!**/*.svgpackages/widget/src/providers/ethereum/finery-wallet-list/custom-wallet-icons/bitgo.svgis excluded by!**/*.svgpackages/widget/src/providers/ethereum/finery-wallet-list/custom-wallet-icons/finoa.svgis excluded by!**/*.svgpackages/widget/src/providers/ethereum/finery-wallet-list/custom-wallet-icons/utila.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (36)
package.jsonpackages/widget/src/components/atoms/virtual-list/index.tsxpackages/widget/src/components/molecules/select-validator/index.tsxpackages/widget/src/pages-dashboard/overview/earn-details/components/provider-selection-card.tsxpackages/widget/src/pages-dashboard/overview/earn-details/earn-details-formatters.tspackages/widget/src/pages-dashboard/overview/earn-details/earn-details-model.tsxpackages/widget/src/pages-dashboard/overview/earn-details/styles.css.tspackages/widget/src/pages-dashboard/overview/earn-page/styles.css.tspackages/widget/src/pages-dashboard/overview/earn-page/utila-select-validator-section.tsxpackages/widget/src/pages-dashboard/overview/earn-page/utila-select-validator-trigger.tsxpackages/widget/src/pages/details/earn-page/components/select-validator-section/select-validator-trigger.tsxpackages/widget/src/pages/details/earn-page/components/select-yield-section/select-yield-reward-details.tsxpackages/widget/src/pages/details/earn-page/styles.css.tspackages/widget/src/translation/English/translations.jsonpackages/widget/src/translation/French/translations.jsonpackages/widget/tests/components/select-validator-section.test.tsxpackages/widget/tests/mocks/delay.tspackages/widget/tests/mocks/external-api-handlers.tspackages/widget/tests/mocks/handlers.tspackages/widget/tests/mocks/legacy-api-handlers.tspackages/widget/tests/mocks/yield-api-handlers.tspackages/widget/tests/pages-dashboard/earn-details-model.test.tsxpackages/widget/tests/pages-dashboard/provider-selection-card.test.tsxpackages/widget/tests/use-cases/deep-links-flow/setup.tspackages/widget/tests/use-cases/external-provider/setup.tspackages/widget/tests/use-cases/gas-warning-flow/setup.tspackages/widget/tests/use-cases/renders-initial-page.test.tsxpackages/widget/tests/use-cases/rwa-kyc-flow.test.tsxpackages/widget/tests/use-cases/select-opportunity.test.tsxpackages/widget/tests/use-cases/sk-wallet.test.tsxpackages/widget/tests/use-cases/staking-flow/setup.tspackages/widget/tests/use-cases/trust-incentive-apy/setup.tspackages/widget/tests/utils/setup.tspackages/widget/vite/vite.config.base.tspackages/widget/vite/vite.node-polyfills.tspnpm-workspace.yaml
💤 Files with no reviewable changes (4)
- packages/widget/src/pages-dashboard/overview/earn-page/utila-select-validator-trigger.tsx
- packages/widget/vite/vite.node-polyfills.ts
- packages/widget/src/pages-dashboard/overview/earn-page/utila-select-validator-section.tsx
- packages/widget/src/pages-dashboard/overview/earn-page/styles.css.ts
| {removableValidator ? ( | ||
| <Box | ||
| aria-label={`Remove ${item.name}`} | ||
| as="button" | ||
| className={styles.providerRemoveButton} | ||
| onClick={() => onRemoveValidator(removableValidator)} | ||
| type="button" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize the remove-button label.
aria-label={Remove ${item.name}} introduces new user-facing copy in English only, so screen-reader output will stay untranslated here (and the same new label pattern exists in select-validator-trigger.tsx). Please move this to i18n with interpolation and add the key to both locale files. As per coding guidelines, "When changing user-facing copy, update both packages/widget/src/translation/English/translations.json and packages/widget/src/translation/French/translations.json."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/widget/src/pages-dashboard/overview/earn-details/components/provider-selection-card.tsx`
around lines 163 - 169, The remove-button aria label in
provider-selection-card.tsx is hardcoded English copy, so it needs to be moved
into i18n with interpolation for the validator name. Update the Box aria-label
to use the shared translation helper, add the new translation key in both
packages/widget/src/translation/English/translations.json and
packages/widget/src/translation/French/translations.json, and apply the same
localization pattern in select-validator-trigger.tsx where the same label is
used.
Source: Coding guidelines
| providerName: firstProvider.name, | ||
| }) | ||
| : (firstProvider?.name ?? ""); | ||
| const displayedProviders = firstProvider ? [firstProvider] : []; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Only the first provider logo is ever rendered.
At Line 218, displayedProviders is reduced to [firstProvider], so the map at Lines 247-255 can never show multiple logos for multi-select cases.
Suggested fix
- const displayedProviders = firstProvider ? [firstProvider] : [];
+ const displayedProviders = providers;Also applies to: 247-255
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/widget/src/pages/details/earn-page/components/select-yield-section/select-yield-reward-details.tsx`
at line 218, The provider logo list is being truncated to a single item in
select-yield-reward-details.tsx, so the rendering loop cannot show multiple
logos for multi-select cases. Update displayedProviders to preserve the full
selected providers collection instead of wrapping only firstProvider, and make
sure the mapping logic that renders the provider logos continues to work with
the complete array in the select-yield-reward-details component.
petar-omni
force-pushed
the
fix/misc
branch
2 times, most recently
from
336d995 to
44b2841
Compare
…tails Show every selected validator in the provider selection card, with per-validator remove controls and a manage validators entry point for multi-select yields. Localize provider status labels, align preferred badge styling with theme tokens, and remove the utila-specific dashboard validator section. Fix multi-select validator list pagination by separating expanded display from explicit load-more requests so the View all action works when only preferred validators are shown. Stabilize browser tests with a fixed mock response delay and disabled animations. Update the node polyfill plugin setup to match current package behavior. Refresh catalog-managed dependency versions and the lockfile.
4 participants
Summary by CodeRabbit